Sprint 3 #28
Sprint 3 #28
Conversation
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
Посмотрите на гитхабе все комментарии к коду (нужно прокрутить вниз страницу там)
Работа проделана огромная:
Readmeхорошо оформлен- Проект задеплоен и отлично работает
- Отлично, что нет
EOFошибок в гите - Хорошая структура папок и файлов
- Отлично, что указываете понятный текст ошибки
- Отлично, что базовый урл вынесен отдельно, чтобы не дублировать его
- Отлично, что ловите возможные ошибки в конце каждого запроса к серверу
- Хорошая логика сокетов и роутинга
но есть некоторые недочеты:
- Когда меняю аватар, то новый не отображается в профиле. Приходится перезагружать страницу, чтобы увидеть изменения
; npx stylelint \"**/*.scss\"не нужно указывать вdev. Так не работает тут- Скрин https://disk.yandex.ru/i/vfc6B_jD6EaIzA В моих чатах есть картинка чата, но она не отображается сейчас у Вас. Вы отображаете там мой аватар. Нужно правильно подключать картинки
- В
scriptsнужно создать отдельные скрипты для запускаESLintиStylelint - Скрин https://disk.yandex.ru/i/Cr9apmidy1Z3kQ Я могу добавить пустое сообщение в чат, а валидация не должна мне этого давать
- Когда я создаю новый чат и начинаю там что-то отправлять, то пустые сообщения отображаются https://disk.yandex.ru/i/k8HFfCixKENMJQ . Но после перезагрузки страницы все ок с этим
JSON.parseможет выкинуть исключение, если данные невалидные. Стоит добавить блокtry/catchдля отлова возможной ошибки- В методе
getнужно обязательно обрабатывать возможныеquery-параметры с помощью функцииqueryStringify. Ее давали в теории
Можно лучше
- Попробуйте избавиться от типов
anyв коде. Хотя бы часть из них можно заменить наunknown - Роутинг должен работать без перезагрузки страницы, как в
SPA-приложениях. В этом суть его создания - Если типизация повторяется в каждом методе (функции), то нужно типизировать сам метод (функцию), а не дублировать типизацию аргументов. Код станет чище
Исправьте, пожалуйста, недочеты и работа будет принята. Пожалуйста, проверьте работоспособность проекта и наличие возможных ошибок в консоли браузера (кнопка F12) перед отправкой на ревью.
Напоминаю, что работа может быть принята только после исправления всех критических замечаний Нужно исправить.
Удачного рефакторинга кода.
package.json
Outdated
| "node": "16.13.2" | ||
| }, | ||
| "scripts": { | ||
| "dev": "vite; npx stylelint \"**/*.scss\"", |
There was a problem hiding this comment.
; npx stylelint \"**/*.scss\" не нужно указывать в dev. Так не работает тут
| "engines": { | ||
| "node": "16.13.2" | ||
| }, | ||
| "scripts": { |
There was a problem hiding this comment.
В scripts нужно создать отдельные скрипты для запуска ESLint и Stylelint
| @@ -0,0 +1,35 @@ | |||
| { | |||
There was a problem hiding this comment.
- Когда меняю аватар, то новый не отображается в профиле. Приходится перезагружать страницу, чтобы увидеть изменения
| @@ -0,0 +1,35 @@ | |||
| { | |||
There was a problem hiding this comment.
- Скрин https://disk.yandex.ru/i/vfc6B_jD6EaIzA В моих чатах есть картинка чата, но она не отображается сейчас у Вас. Вы отображаете там мой аватар. Нужно правильно подключать картинки
src/utils/HTTPTransport.ts
Outdated
| } | ||
|
|
||
| export default class HTTPTransport { | ||
| static API_URL = 'https://ya-praktikum.tech/api/v2' |
There was a problem hiding this comment.
- Отлично, что базовый урл вынесен отдельно, чтобы не дублировать его
| Быстро понять статус проекта помогают бейджи на «Гитхабе». Иногда разработчики ограничиваются парой бейджев, которые сообщат о статусе тестов кода: | ||
|
|
||
|  | ||
| ## Описание | ||
|
|
There was a problem hiding this comment.
- Скрин https://disk.yandex.ru/i/Cr9apmidy1Z3kQ Я могу добавить пустое сообщение в чат, а валидация не должна мне этого давать
| - Как применять? | ||
|
|
||
| ## Бейджи | ||
|
|
There was a problem hiding this comment.
- Когда я создаю новый чат и начинаю там что-то отправлять, то пустые сообщения отображаются https://disk.yandex.ru/i/k8HFfCixKENMJQ . Но после перезагрузки страницы все ок с этим
src/utils/WSTransport.ts
Outdated
| socket.addEventListener('close', () => this.emit(WSTransportEvents.Close)) | ||
| socket.addEventListener('error', (e) => this.emit(WSTransportEvents.Error, e)) | ||
| socket.addEventListener('message', (message) => { | ||
| const data = JSON.parse(message.data) |
There was a problem hiding this comment.
JSON.parse может выкинуть исключение, если данные невалидные. Стоит добавить блок try/catch для отлова возможной ошибки
src/utils/HTTPTransport.ts
Outdated
| } | ||
|
|
||
| // create chat '/chats', { title: title } | ||
| public post<Response = void>(path: string, data?: unknown): Promise<Response> { |
There was a problem hiding this comment.
Можно лучше
Если типизация повторяется в каждом методе (функции), то нужно типизировать сам метод (функцию), а не дублировать типизацию аргументов. Код станет чище
// создаем тип метода
type HTTPMethod<Response = void> = (url: string, options?: unknown) => Promise<Response >
export class HTTPTransport {
// используем тип и удаляем дублирование в аргументах
get: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.GET}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
put: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.PUT}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
post: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.POST}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
delete: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.DELETE}, options.timeout)
)И в request не нужно будет передавать тип. Он автоматом определится через Promise
src/utils/HTTPTransport.ts
Outdated
| } | ||
|
|
||
| public get<Response>(path = '/'): Promise<Response> { | ||
| return this.request<Response>(this.endpoint + path) |
There was a problem hiding this comment.
В методе get нужно обязательно обрабатывать возможные query-параметры с помощью функции queryStringify. Ее давали в теории
No description provided.